Subject: fix for vulnerability CVE-2011-3343 for OpenTTD 0.6.0 - 0.6.3 (Multiple buffer overflows in validation of external data)
From: OpenTTD developer team <info@openttd.org>
Origin: backport, https://github.com/OpenTTD/OpenTTD/commit/6c7cbb1 https://github.com/OpenTTD/OpenTTD/commit/65637d8 https://github.com/OpenTTD/OpenTTD/commit/73624ab
Bug: https://github.com/OpenTTD/OpenTTD/issues/4746 https://github.com/OpenTTD/OpenTTD/issues/4747

In multiple places external data isn’t properly checked before allocating
memory, which could lead to buffer overflows and arbitrary code execution.

These bugs are only exploitable locally by providing OpenTTD with
invalid/manipulated images, sounds or fonts. This means an attacker
either needs local access or has to trick an user into loading a
manipulated image into OpenTTD. This is especially a concern with
BMP files loaded as heightmaps.

All except one vulnerability are caused by improper validation of input
data prior to allocating memory buffers. It is possible to force allocation
of a too small buffer and thus out-of-bounds writes by causing an integer
overflow. Additionally in RLE-compressed BMP images, it is possible to
write arbitrary data outside the allocated buffer.

No patch for releases before 0.3.1 is provided, as this versions are
unsupported since a long time and would require larger changes not worth
the effort.


diff --git src/bmp.cpp src/bmp.cpp
index c5677fb..4ee1964 100644
--- src/bmp.cpp
+++ src/bmp.cpp
@@ -135,6 +135,7 @@ static inline bool BmpRead4Rle(BmpBuffer *buffer, BmpInfo *info, BmpData *data)
 			switch (c) {
 			case 0: // end of line
 				x = 0;
+				if (y == 0) return false;
 				pixel = &data->bitmap[--y * info->width];
 				break;
 			case 1: // end of bitmap
@@ -145,7 +146,7 @@ static inline bool BmpRead4Rle(BmpBuffer *buffer, BmpInfo *info, BmpData *data)
 			case 2: // delta
 				x += ReadByte(buffer);
 				i = ReadByte(buffer);
-				if (x >= info->width || (y == 0 && i > 0)) return false;
+				if (x >= info->width || i > y) return false;
 				y -= i;
 				pixel = &data->bitmap[y * info->width + x];
 				break;
@@ -218,6 +219,7 @@ static inline bool BmpRead8Rle(BmpBuffer *buffer, BmpInfo *info, BmpData *data)
 			switch (c) {
 			case 0: // end of line
 				x = 0;
+				if (y == 0) return false;
 				pixel = &data->bitmap[--y * info->width];
 				break;
 			case 1: // end of bitmap
@@ -228,13 +230,16 @@ static inline bool BmpRead8Rle(BmpBuffer *buffer, BmpInfo *info, BmpData *data)
 			case 2: // delta
 				x += ReadByte(buffer);
 				i = ReadByte(buffer);
-				if (x >= info->width || (y == 0 && i > 0)) return false;
+				if (x >= info->width || i > y) return false;
 				y -= i;
 				pixel = &data->bitmap[y * info->width + x];
 				break;
 			default: // uncompressed
-				if ((x += c) > info->width) return false;
-				for (i = 0; i < c; i++) *pixel++ = ReadByte(buffer);
+				for (i = 0; i < c; i++) {
+					if (EndOfBuffer(buffer) || x >= info->width) return false;
+					*pixel++ = ReadByte(buffer);
+					x++;
+				}
 				/* Padding for 16 bit align */
 				SkipBytes(buffer, c % 2);
 				break;
diff --git src/fontcache.cpp src/fontcache.cpp
index 4774279..74d0024 100644
--- src/fontcache.cpp
+++ src/fontcache.cpp
@@ -422,6 +422,9 @@ static bool GetFontAAState(FontSize size)
 	width  = max(1, slot->bitmap.width + (size == FS_NORMAL));
 	height = max(1, slot->bitmap.rows  + (size == FS_NORMAL));
 
+	/* Limit glyph size to prevent overflows later on. */
+	if (width > 256 || height > 256) error("Font glyph is too large");
+
 	/* FreeType has rendered the glyph, now we allocate a sprite and copy the image into it */
 	sprite.data = CallocT<SpriteLoader::CommonPixel>(width * height);
 	sprite.width = width;
diff --git src/heightmap.cpp src/heightmap.cpp
index c04ebd8..e2645a5 100644
--- src/heightmap.cpp
+++ src/heightmap.cpp
@@ -139,6 +139,14 @@ static bool ReadHeightmapPNG(char *filename, uint *x, uint *y, byte **map)
 		return false;
 	}
 
+	/* Check if image dimensions don't overflow a size_t to avoid memory corruption. */
+	if ((uint64)info_ptr->width * info_ptr->height >= (size_t)-1) {
+		ShowErrorMessage(STR_ERROR_HEIGHTMAP_TOO_LARGE, STR_PNGMAP_ERROR, 0, 0);
+		fclose(fp);
+		png_destroy_read_struct(&png_ptr, &info_ptr, NULL);
+		return false;
+	}
+
 	if (map != NULL) {
 		*map = MallocT<byte>(info_ptr->width * info_ptr->height);
 
@@ -248,6 +256,14 @@ static bool ReadHeightmapBMP(char *filename, uint *x, uint *y, byte **map)
 		return false;
 	}
 
+	/* Check if image dimensions don't overflow a size_t to avoid memory corruption. */
+	if ((uint64)info.width * info.height >= (size_t)-1 / (info.bpp == 24 ? 3 : 1)) {
+		ShowErrorMessage(STR_ERROR_HEIGHTMAP_TOO_LARGE, STR_BMPMAP_ERROR, 0, 0);
+		fclose(f);
+		BmpDestroyData(&data);
+		return false;
+	}
+
 	if (map != NULL) {
 		if (!BmpReadBitmap(&buffer, &info, &data)) {
 			ShowErrorMessage(STR_BMPMAP_ERR_IMAGE_TYPE, STR_BMPMAP_ERROR, 0, 0);
diff --git src/lang/english.txt src/lang/english.txt
index 19b4e35..35402ae 100644
--- src/lang/english.txt
+++ src/lang/english.txt
@@ -1585,6 +1585,8 @@ STR_PNGMAP_ERR_MISC                                             :{WHITE}...somet
 STR_BMPMAP_ERROR                                                :{WHITE}Cannot load landscape from BMP...
 STR_BMPMAP_ERR_IMAGE_TYPE                                       :{WHITE}...could not convert image type.
 
+STR_ERROR_HEIGHTMAP_TOO_LARGE                                   :{WHITE}... image is too large
+
 ##id 0x0800
 STR_0800_COST                                                   :{TINYFONT}{RED}Cost: {CURRENCY}
 STR_0801_COST                                                   :{RED}Cost: {CURRENCY}
diff --git src/sound.cpp src/sound.cpp
index 9fed4a6..fef60b5 100644
--- src/sound.cpp
+++ src/sound.cpp
@@ -117,7 +117,8 @@ static bool SetBankSource(MixerChannel *mc, const FileEntry *fe)
 {
 	assert(fe != NULL);
 
-	if (fe->file_size == 0) return false;
+	/* Check for valid sound size. */
+	if (fe->file_size == 0 || fe->file_size > ((size_t)-1) - 2) return false;
 
 	int8 *mem = MallocT<int8>(fe->file_size);
 	if (mem == NULL) return false;
diff --git src/sound/win32_s.cpp src/sound/win32_s.cpp
index 386c6f4..2c184c6 100644
--- src/sound/win32_s.cpp
+++ src/sound/win32_s.cpp
@@ -60,6 +60,7 @@ static void CALLBACK waveOutProc(HWAVEOUT hwo, UINT uMsg, DWORD_PTR dwInstance,
 	wfex.nAvgBytesPerSec = wfex.nSamplesPerSec * wfex.nBlockAlign;
 
 	_bufsize = GetDriverParamInt(parm, "bufsize", (GB(GetVersion(), 0, 8) > 5) ? 2048 : 1024);
+	_bufsize = min(_bufsize, 65535);
 
 	if (waveOutOpen(&_waveout, WAVE_MAPPER, &wfex, (DWORD_PTR)&waveOutProc, 0, CALLBACK_FUNCTION) != MMSYSERR_NOERROR)
 		return "waveOutOpen failed";
diff --git src/spriteloader/png.cpp src/spriteloader/png.cpp
index 363dce4..26440bf 100644
--- src/spriteloader/png.cpp
+++ src/spriteloader/png.cpp
@@ -102,7 +102,17 @@ static bool LoadPNG(SpriteLoader::Sprite *sprite, const char *filename, uint32 i
 
 		sprite->height = info_ptr->height;
 		sprite->width  = info_ptr->width;
+		/* Check if sprite dimensions aren't larger than what is allowed in GRF-files. */
+		if (info_ptr->height > 255 || info_ptr->width > 65535) {
+			png_destroy_read_struct(&png_ptr, &info_ptr, &end_info);
+			return false;
+		}
 		sprite->data = CallocT<SpriteLoader::CommonPixel>(sprite->width * sprite->height);
+	} else if (sprite->height != info_ptr->height || sprite->width != info_ptr->width) {
+		/* Make sure the mask image isn't larger than the sprite image. */
+		DEBUG(misc, 0, "Ignoring mask for SpriteID %d as it isn't the same dimension as the masked sprite", id);
+		png_destroy_read_struct(&png_ptr, &info_ptr, &end_info);
+		return true;
 	}
 
 	bit_depth  = png_get_bit_depth(png_ptr, info_ptr);
@@ -110,6 +120,7 @@ static bool LoadPNG(SpriteLoader::Sprite *sprite, const char *filename, uint32 i
 
 	if (mask && (bit_depth != 8 || color_type != PNG_COLOR_TYPE_PALETTE)) {
 		DEBUG(misc, 0, "Ignoring mask for SpriteID %d as it isn't a 8 bit palette image", id);
+		png_destroy_read_struct(&png_ptr, &info_ptr, &end_info);
 		return true;
 	}
 

